write chunks in TextParserStreamer #3025
Conversation
github-actions
bot
added
no-match-files
category: GGUF
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the TextParserStreamer to return incremental delta messages in the write() callback instead of accumulated messages, while still maintaining accumulated message access via get_parsed_message(). The key changes update the reasoning parser implementation to properly handle delta messages and modify the concatenation logic in the streamer.
- Modified
TextParserStreamerto write delta messages instead of accumulated messages - Updated
ReasoningIncrementalParserto populate message fields for incremental output - Added
concatenate_json_containers()helper function for message accumulation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/python_tests/test_parsers.py | Updated tests to use get_parsed_message() for assertions and removed manual message accumulation in custom streamers |
| src/cpp/src/text_streamer.cpp | Added concatenate_json_containers() function and modified write() to return delta messages while maintaining internal accumulation |
| src/cpp/src/parsers.cpp | Refactored ReasoningIncrementalParser to populate content field in delta messages and removed keep_original_content logic from helper methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pavel-esir
force-pushed
the
write_chunks_during_parse
branch
from
9b7cd5c to
a41f0d0
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
tests/python_tests/test_parsers.py:1
- Assignment to
reason_strloses previously accumulated reasoning content. The variable should be appended to, not replaced, to preserve partial reasoning text from previous chunks.
# Copyright (C) 2023-2025 Intel Corporation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pavel-esir
force-pushed
the
write_chunks_during_parse
branch
2 times, most recently
from
d71fc3f to
4017fb5
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
bot
added
the
category: CPP API
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Extract reasoning content between tags | ||
| message["reasoning_content"] = std::string(txt_chunk.substr(open_idx + m_open_tag.size(), | ||
| close_idx - (open_idx + m_open_tag.size()))); | ||
| message["reasoning_content"] = std::string(txt_chunk.substr(open_idx + m_open_tag.size(), close_idx - (open_idx + m_open_tag.size()))); |
There was a problem hiding this comment.
[nitpick] The calculation close_idx - (open_idx + m_open_tag.size()) is duplicated from the removed line. Consider extracting this to a variable for clarity, e.g., size_t reasoning_length = close_idx - (open_idx + m_open_tag.size());
| message["reasoning_content"] = std::string(txt_chunk.substr(open_idx + m_open_tag.size(), close_idx - (open_idx + m_open_tag.size()))); | |
| size_t reasoning_length = close_idx - (open_idx + m_open_tag.size()); | |
| message["reasoning_content"] = std::string(txt_chunk.substr(open_idx + m_open_tag.size(), reasoning_length)); |
| // Intentionally clear delta_text: no delta content is returned to the user during this phase | ||
| // (we are waiting for the <think> tag to be fully detected in the cache). |
There was a problem hiding this comment.
[nitpick] The comment above this line describes why delta_text is cleared, but it should be updated to reflect that this intentional clearing is part of the caching strategy while waiting for the full <think> tag.
| // Intentionally clear delta_text: no delta content is returned to the user during this phase | |
| // (we are waiting for the <think> tag to be fully detected in the cache). | |
| // Intentionally clear delta_text as part of the caching strategy: | |
| // no delta content is returned to the user during this phase because we are | |
| // accumulating partial data in m_text_cache until the full <think> tag is detected. |
| * @param message JsonContainer to store parsed results and reasoning metadata | ||
| * @param delta_text New text chunk to be processed in this step | ||
| * @param delta_tokens Optional vector of new token IDs to be processed | ||
| * @return std::string Filtered text with reasoning content processed according to configuration |
There was a problem hiding this comment.
Update doc. Replace message -> delta_message in @param message JsonContainer to store parsed results and reasoning metadata
2 participants
Description
Return DeltaMessage in incremental
TextStreamerParserinstead of accumulated msg. But accumulated is still available viaget_parsed_message()CVS-CVS-176146
Fixes #(issue)
Checklist: